Skip to content

Ee 1082 update enverus endpoints#16

Open
mricher2000 wants to merge 3 commits intomasterfrom
EE-1082-OI-api-integration-update-braavos
Open

Ee 1082 update enverus endpoints#16
mricher2000 wants to merge 3 commits intomasterfrom
EE-1082-OI-api-integration-update-braavos

Conversation

@mricher2000
Copy link
Copy Markdown
Collaborator

@mricher2000 mricher2000 commented May 2, 2024

Ticket

EE-1082

Changes

  • Updating the endpoints to use versioning
  • Removing the payment endpoint as it has been deprecated by Enverus & we aren't using it anywhere

NOTE: a new base_url is also being used. It has been set for aux1 here

Note: the base_url will need to be updated for the other test environments & production

Enverus API docs here

Definition of Done

  • Pull request is up to date with the destination branch
  • Commits were squashed
  • All Tests are passing
  • At least one code review
  • Screen-Review passed and logged.

@mricher2000 mricher2000 force-pushed the EE-1082-OI-api-integration-update-braavos branch from 0b868e9 to 0c53d3f Compare May 2, 2024 19:19
@mricher2000 mricher2000 changed the title Ee 1082 oi api integration update braavos Ee 1082 update enverus endpoints May 2, 2024
@mricher2000 mricher2000 marked this pull request as ready for review May 2, 2024 19:41
@mricher2000 mricher2000 requested review from dbaron-ft and lineshFT May 2, 2024 19:41
@dbaron-ft
Copy link
Copy Markdown

Are the OI API docs available either on Enverus site or somewhere in Confluence? Might not turn up in searches otherwise.

Another possibility would be to update engineering docs/readme in this project to point to the google doc.

Copy link
Copy Markdown

@dbaron-ft dbaron-ft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple questions.

Overall seems to be a mix of V1 and V2 in use at the same time, will these work together or does it require being all in on V2?

class Invoice < Base
def index(opts = {})
request(:get, "/supplier/#{supplier_uuid}/invoices/page", opts)
request(:get, "/#{OpenInvoice::Configure::VERSION_1}/supplier/#{supplier_uuid}/invoices/page", opts)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these VERSION_1 whereas the base class is VERSION_2?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, some of Enverus's endpoints are on V1, and others are on V2 (currently only buyer & supplier).

def buyer
@buyer ||= ::OpenInvoice::Entities::Buyer.new(supplier_uuid)
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen payment API used by Quickbooks integration. Is it possible that Braavos calls payment for all the integrations and might error out if this is removed? (just wondering if this should be a no-op rather than removed)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm it is possible @dbaron-ft. We have no payments in our braavos db where the source is oi, but looking at the data import job in braavos, there is a Generics::Importers::Base class that is used to imort payments.. which makes me think that maybe it tries to call this endpoint, even for OI?

I can leave it in & add a comment if that makes more sense?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more context, I met with the OI API team last week and asked them about this endpoint. The investigated and confirmed that they have nothing on their end related to a payment endpoint currently in production or being developed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants